fix(app): keep JSON-column filter keys in JSON form when another filter is added by ralaruri · Pull Request #3641 · ClickHouse/ClickStack

ralaruri · 2026-11-12T20:24:17Z

Summary

With a native JSON column as the source's attributes column, adding a second filter breaks the first one. The table fails with Code 43: First argument for function 'arrayElement' must be array, got 'JSON'.

Every filter change re-escapes all keys in the filter state. toClickHouseKeyExpression sees a dotted key like message.email, assumes a Map column, and writes message['email']. It only receives column names, not types. The newest filter is still in its original JSON form, so only the earlier one breaks.

This passes the table's JSON column names (taken from the column types the search page already loads) into escapeFilterStateKeys and toQuotedClickHouseKeyExpression. Keys under a JSON column are now written as toString(`message`.`email`), the form the row panel already produces. Map columns are unchanged. Array subscripts in a path (`events`[1]) stay subscripts. The facets panel builds its keys the same way, through the existing useJsonColumns hook.

Tests: unit tests for the key function, plus a hook test where a JSON-key filter exists and another filter is added. They fail on main and pass here. tsc and the related jest suites pass.

Screenshots or video

Before	After

How to reproduce

Starting point: ClickStack Local (docker run -p 8080:8080 -p 8000:8000 -p 8123:8123 docker.clickhouse.com/clickhouse/clickstack-local) with this fake table, created with clickhouse-client in the container:

CREATE DATABASE demo;
CREATE TABLE demo.events_json (
  ingested_at DateTime64(3) DEFAULT now64(3), organization_id UUID,
  occurred_at DateTime64(3), source_name LowCardinality(String), message JSON
) ENGINE = MergeTree ORDER BY (organization_id, occurred_at);
INSERT INTO demo.events_json (organization_id, occurred_at, source_name, message)
SELECT generateUUIDv4(), now64(3) - toIntervalSecond(number), 'EMAIL_SERVICE',
  concat('{"email":"user', toString(number % 7), '@example.test","owner":{"profile":{"contact":{"email":"owner', toString(number % 7), '@example.test"}}}}')::JSON
FROM numbers(200);

Add a Logs source: table demo.events_json, Timestamp Column occurred_at, Log Attributes Expression message, Default Select ingested_at, organization_id, message.

On main:

Open the source, set the range to the last hour, click Run.
Expand a row, open Column Values, expand message, hover email, click the filter icon. Rows still show.
Add a second filter: owner → profile → contact → email.
The table shows the Code 43 error. In </>, the WHERE clause has message['email'].

On this branch:

Same steps. Both filters apply and rows show.
In </>, both are written as toString(`message`.`…`).

How to test on Vercel preview

N/A. It needs a source with a native JSON column, which the demo server probably doesn't have.

References

Related issue: Search sidebar JSON resource filters use map access #2549
Related PRs: fix: handle native JSON search filters #2551, fix(app): serialize JSON column filters with dot access (HDX-5085) #2864

changeset-bot · 2026-11-12T20:24:24Z

🦋 Changeset detected

Latest commit: eeb05a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/api	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2026-11-12T20:24:24Z

@ralaruri is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2026-11-12T20:24:36Z

Hi @ralaruri, thanks for the pull request!

Before we review code from a first-time contributor, a maintainer vouches for them, and you're not on our list yet. There's nothing else to file and this PR stays open — a maintainer will look at it, usually within a day or two, and vouch for you right here if it's a genuine attempt at the problem.

To make that quick, check the PR description has:

The issue it addresses (ideally one labelled Accepting PR) and why the change is needed
Steps to reproduce the problem and verify the fix, starting from https://play.hyperdx.io or yarn app:dev:local connected to the demo server
Before/after screenshots if anything in the UI changed

More detail in our contributing guide.

github-actions · 2026-11-12T20:25:54Z

+      inputSourceColumns
+        ? new Set(
+            inputSourceColumns
+              .filter(c => c.type === 'JSON' || c.type.startsWith('JSON('))


🔵 minor — JSON column detection re-implements an existing type helper

This hand-written c.type === 'JSON' || c.type.startsWith('JSON(') predicate repeats DBRowDataPanel.tsx:414 and the existing helpers filterColumnMetaByType(columns, [JSDataType.JSON]) and convertCHDataTypeToJSType in common-utils (core/metadata, clickhouse/index.ts:119), which useJsonColumns already uses. Use filterColumnMetaByType(inputSourceColumns, [JSDataType.JSON]) so JSON detection has one source of truth.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-11-12T20:25:55Z

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.

🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.

Anything about surrounding code, or asking you to widen the change (hoist a helper,
dedupe with another file, fix other call sites): never fix here, whatever the
severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

2 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 1 minor

1 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

🟠 packages/app/src/components/DBSearchPageFilters/hooks.ts:229 — "Load more" still re-escapes JSON filter keys to Map brackets, hitting the same Code 43 error → In exact mode, loadMoreFacetsForKey builds its filters with escapeFilterStateKeys(strippedFilterState, knownColumns) and no JSON columns. filterState holds clean keys such as message.email here (the persisted form after unescapeFilterStateKeys), so this still emits message['email'], and the Load more query fails with arrayElement must be array, got 'JSON'. Pass new Set(jsonColumns ?? []) (the useJsonColumns result already in scope at line 105) to this call. For consistency, also pass it to the toQuotedClickHouseKeyExpression calls at lines 173 and 219.

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

…re-escaped Adding a second filter re-escaped every key in the filter state. toClickHouseKeyExpression treated a dotted key under a native JSON column as a Map key and rewrote it to col['key'], which ClickHouse rejects: 'First argument for function arrayElement must be array, got JSON'. Pass the table's JSON column names into escapeFilterStateKeys and toQuotedClickHouseKeyExpression, so keys under a JSON column are written as toString(`col`.`a`.`b`), the form the row panel already produces. Array subscripts in a path (`col`.`a`[1]) stay subscripts. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

github-actions · 2026-11-12T20:31:43Z

Deep Review

The change is a well-scoped, well-tested fix: it threads a jsonColumns set into filter-key escaping so dotted keys under a native ClickHouse JSON column are re-serialized as toString(\col`.`a`)rather than the Map formcol['a'] that ClickHouse rejects (Code 43). The core transform in toQuotedClickHouseKeyExpressionguards correctly (skips already-quoted, bracket, and function-call keys), escapes each path segment withSqlString.escapeId, keeps digit-only array subscripts outside the quoted name, and is covered by direct unit tests plus a hook round-trip test. No injection path survives: every identifier segment is backtick-escaped and subscripts are constrained to [\d+]`. No P0/P1 production breakage is introduced by this diff.

✅ No critical issues found.

🟡 P2 — recommended

packages/app/src/components/DBSearchPageFilters.tsx:1263 — The new facets-panel wiring that passes jsonColumnSet into toQuotedClickHouseKeyExpression, and the inline JSON-type detection in DBSearchPage.tsx (including the JSON(...) parametrized form), are new behavior exercised only indirectly; the added unit/hook tests cover the pure key function and useSearchPageFilterState but not these two call sites.
- Fix: Add a test asserting the facets panel builds JSON-column sqlKeys in toString(\col`.`key`)form, and a case covering a parametrizedJSON(max_dynamic_paths=...)` column type feeding the detection memo.

🔵 P3 nitpicks (2)

packages/app/src/DBSearchPage.tsx:1440 — JSON-column detection is reimplemented inline as c.type === 'JSON' || c.type.startsWith('JSON('), duplicating the shared classification used by the useJsonColumns hook (convertCHDataTypeToJSType) that feeds the facets panel; the two can drift since the shared path also unwraps LowCardinality/Nullable wrappers (not reachable today, but the two sources of truth escape keys for the same table).
- Fix: Derive both sets from one shared helper so the filter-state and facets paths classify JSON columns identically.
packages/app/src/components/DBSearchPageFilters/utils.ts:333 — Splitting the cleaned path on . means a JSON key that literally contains a dot round-trips ambiguously (it is re-split into two quoted segments), a pre-existing limitation of the clean-form key representation that this branch now also inherits for JSON paths.
- Fix: Note the single-level-key assumption in the function comment, or guard the case if dotted JSON keys are expected.

Reviewers (6): correctness, kieran-typescript, maintainability, testing, security, project-standards.

Testing gaps: No coverage for the facets-panel jsonColumnSet path in DBSearchPageFilters.tsx; no coverage for JSON(...) parametrized-type detection in the DBSearchPage.tsx memo.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

ralaruri requested a review from a team as a code owner November 12, 2026 20:24

github-actions Bot added external needs-vouch Author needs a maintainer to vouch for them labels Nov 12, 2026

ralaruri mentioned this pull request Nov 12, 2026

fix(app): address JSON array elements by index in row-panel filters #3642

Draft

github-actions Bot reviewed Nov 12, 2026

View reviewed changes

ralaruri force-pushed the fix/json-filter-keys branch from ed19bbb to eeb05a5 Compare November 12, 2026 20:26

Navigation Menu

Repository navigation

fix(app): keep JSON-column filter keys in JSON form when another filter is added - #3641

fix(app): keep JSON-column filter keys in JSON form when another filter is added#3641
ralaruri wants to merge 1 commit into
ClickHouse:mainfrom
ralaruri:fix/json-filter-keys

ralaruri commented Nov 12, 2026

Uh oh!

changeset-bot Bot commented Nov 12, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

github-actions Bot Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ralaruri commented Nov 12, 2026

Summary

Screenshots or video

How to reproduce

How to test on Vercel preview

References

Uh oh!

changeset-bot Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

github-actions Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 12, 2026

PR Review

Findings outside the changed lines

Uh oh!

github-actions Bot commented Nov 12, 2026

Deep Review

🟡 P2 — recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot Bot commented Nov 12, 2026 •

edited

Loading